[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wjyrich

The full list of commands accepted by this bot can be found here.

Reviewer's Guide

Adds an explicit supported state to dock applets, wires MultiTaskView’s support to compositor/KWin effect availability, and updates the DBus proxy to ignore unsupported applets for visibility and plugin listing, ensuring visibility and signals reflect both user choice and platform support.

Sequence diagram for compositor changes affecting MultiTaskView visibility

sequenceDiagram
    participant DWindowManagerHelper
    participant MultiTaskView
    participant DAppletDock as BaseDockApplet

    DWindowManagerHelper->>MultiTaskView: hasCompositeChanged(newHasComposite)
    activate MultiTaskView
    MultiTaskView-->>MultiTaskView: Q_EMIT visibleChanged()
    MultiTaskView->>MultiTaskView: setSupported(m_kWinEffect && hasComposite())
    alt supported state unchanged
        MultiTaskView-->>MultiTaskView: return (no signal change)
    else supported state changed
        MultiTaskView->>BaseDockApplet: setSupported(bool supported)
        activate BaseDockApplet
        BaseDockApplet-->>BaseDockApplet: oldEffectiveVisible = visible()
        BaseDockApplet-->>BaseDockApplet: m_isSupported = supported
        BaseDockApplet-->>BaseDockApplet: Q_EMIT supportedChanged()
        alt effective visibility changed
            BaseDockApplet-->>BaseDockApplet: Q_EMIT visibleChanged()
        end
        deactivate BaseDockApplet
    end
    deactivate MultiTaskView

Sequence diagram for DockDBusProxy filtering unsupported dock applets

sequenceDiagram
    participant Client
    participant DockDBusProxy
    participant DockApplet1
    participant DockApplet2

    Client->>DockDBusProxy: updateDockPluginsVisible(pluginsVisible)
    activate DockDBusProxy
    loop for each dockApplet in dockApplets(parent)
        alt dockApplet is unsupported
            DockDBusProxy->>DockApplet1: isSupported()
            DockApplet1-->>DockDBusProxy: false
            DockDBusProxy-->>DockDBusProxy: continue (skip applet)
        else dockApplet is supported
            DockDBusProxy->>DockApplet2: isSupported()
            DockApplet2-->>DockDBusProxy: true
            DockDBusProxy->>DockApplet2: dockItemInfo()
            DockApplet2-->>DockDBusProxy: DockItemInfo
            DockDBusProxy-->>DockDBusProxy: apply pluginsVisible[itemKey]
        end
    end
    deactivate DockDBusProxy

    Client->>DockDBusProxy: plugins()
    activate DockDBusProxy
    loop for each dockApplet in dockApplets(parent)
        alt dockApplet is unsupported
            DockDBusProxy->>DockApplet1: isSupported()
            DockApplet1-->>DockDBusProxy: false
            DockDBusProxy-->>DockDBusProxy: skip appending
        else dockApplet is supported
            DockDBusProxy->>DockApplet2: isSupported()
            DockApplet2-->>DockDBusProxy: true
            DockDBusProxy->>DockApplet2: dockItemInfo()
            DockApplet2-->>DockDBusProxy: DockItemInfo
            DockDBusProxy-->>DockDBusProxy: iteminfos.append(info)
        end
    end
    DockDBusProxy-->>Client: DockItemInfos (supported applets only)
    deactivate DockDBusProxy

Class diagram for dock applet support and visibility model

classDiagram
    class DAppletDockPrivate {
        bool m_visible
        bool m_isSupported
    }

    class DAppletDock {
        +bool visible()
        +void setVisible(bool visible)
        +bool isSupported()
        +void setSupported(bool supported)
        +DockItemInfo dockItemInfo()
        +void init()
        <<signals>>
        +void visibleChanged()
        +void supportedChanged()
    }

    class MultiTaskView {
        +MultiTaskView(QObject parent)
        +bool init()
        +DockItemInfo dockItemInfo()
        -bool m_kWinEffect
        -QScopedPointer multitaskview
    }

    class DockDBusProxy {
        +QRect geometry()
        +void updateDockPluginsVisible(QVariantMap pluginsVisible)
        +DockItemInfos plugins()
    }

    class DWindowManagerHelper {
        +static DWindowManagerHelper instance()
        +bool hasComposite()
        <<signals>>
        +void hasCompositeChanged(bool hasComposite)
    }

    class DockItemInfo {
        QString displayName
        QString itemKey
        QString settingKey
        bool visible
        QString dccIcon
    }

    DAppletDockPrivate <.. DAppletDock : uses
    DAppletDock *-- DAppletDockPrivate : owns
    MultiTaskView --|> DAppletDock
    DockDBusProxy ..> DAppletDock : iterates_dockApplets
    DWindowManagerHelper ..> MultiTaskView : notifies_compositor_change
    DAppletDock ..> DockItemInfo : returns
    MultiTaskView ..> DockItemInfo : returns

File-Level Changes

deepin pr auto review

这份代码diff主要实现了为Dock插件增加一个isSupported属性，用于控制插件是否被支持（例如根据系统环境判断），并在逻辑上将其与visible属性结合，决定插件是否真正显示。以下是对代码的详细审查和改进建议：

1. 语法逻辑审查

整体逻辑：代码逻辑基本正确。通过引入m_isSupported标志，将"系统环境是否支持"与"用户是否设置可见"解耦，最终通过visible()方法结合两者决定实际可见性。

潜在问题与改进：

• setSupported 中的信号发射顺序：
  在 dappletdock.cpp 的 setSupported 函数中：

  void DAppletDock::setSupported(bool supported)
  {
      // ...
      d->m_isSupported = supported;
      Q_EMIT supportedChanged(); // 发射 supportedChanged
  
      if (oldEffectiveVisible != this->visible()) {
          Q_EMIT visibleChanged(); // 如果可见性改变，发射 visibleChanged
      }
  }

  建议：逻辑上是合理的，先发射属性本身的信号，再发射受其影响的属性信号。这符合属性依赖关系的处理习惯，无需修改。

• MultiTaskView::init 中的重复设置：

  bool MultiTaskView::init()
  {
      setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
      setVisible(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
      DAppletDock::init();
      return true;
  }

  问题：这里显式调用了 setVisible。虽然逻辑上是为了初始化状态，但 visible 属性现在依赖于 m_isSupported。如果在后续配置加载中（例如 DAppletDock::init 内部）从配置文件读取了用户设置的 visible 状态，这里的 setVisible 调用可能会被覆盖，或者覆盖掉配置文件的值。
  建议：检查 DAppletDock::init 的实现。如果 init 中会从配置恢复 m_visible，那么这里的 setVisible 是多余的。如果目的是"如果不支持，则强制不可见"，那么应该只设置 setSupported，让 visible() 自动返回 false，或者确保配置加载逻辑能正确处理"不支持"的情况。
  改进方案：如果配置文件保存的是用户的"意图"（即用户想让它显示），那么代码应改为：

  bool MultiTaskView::init()
  {
      // 仅设置支持状态，不强制覆盖用户配置的可见性
      setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
      DAppletDock::init();
      return true;
  }

2. 代码质量审查

• Lambda 表达式中的逻辑重复：
  在 multitaskview.cpp 中：

  connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, [this]() {
      Q_EMIT visibleChanged();
      setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
  });

  以及后面 m_kWinEffect 变化时的处理：

  if (kWinEffect != m_kWinEffect) {
      m_kWinEffect = kWinEffect;
      Q_EMIT visibleChanged();
      setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
  }

  问题：Q_EMIT visibleChanged() 在 setSupported 内部（如果导致可见性变化）也会触发。这里手动发射了一次，setSupported 内部可能还会再发射一次。
  建议：移除 Lambda 和判断块中的 Q_EMIT visibleChanged()，完全依赖 setSupported 内部的逻辑来触发信号，保持逻辑单一入口。
  改进代码：

  // 改进 1
  connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, [this]() {
      setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
  });
  
  // 改进 2
  if (kWinEffect != m_kWinEffect) {
      m_kWinEffect = kWinEffect;
      setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
  }

• dockItemInfo 的实现：

  DockItemInfo MultiTaskView::dockItemInfo()
  {
      // ...
      info.visible = visible(); // 使用了 visible() 而不是原来的判断逻辑
      // ...
  }

  评价：这是一个很好的改进。它封装了逻辑，使得 DockItemInfo 的可见性状态与对象的实际属性状态保持一致，避免了在不同地方重复判断 m_kWinEffect && ... 的逻辑。

3. 代码性能审查

• 遍历检查：
  在 dockdbusproxy.cpp 中：

  for (auto *dockApplet : dockApplets(parent())) {
      if (!dockApplet->isSupported()) {
          continue;
      }
      // ...
  }

  分析：增加了一次虚函数调用 isSupported()。考虑到 Dock 插件数量通常很少（一般 < 20），这个性能开销可以忽略不计。代码清晰度优于微小的性能损耗。

4. 代码安全审查

• 空指针检查：
  在 dockdbusproxy.cpp 中，dockApplets(parent()) 返回的容器中的元素 dockApplet 在使用前进行了 isSupported() 检查。
  建议：虽然 dockApplets 通常返回有效对象，但为了防御性编程，建议确认 dockApplet 是否可能为 nullptr。如果可能，应增加判断：

  if (!dockApplet || !dockApplet->isSupported()) {
      continue;
  }

• Lambda 捕获：
  在 multitaskview.cpp 中，Lambda 捕获了 this 指针。由于这是在 MultiTaskView 构造函数中连接到单例 DWindowManagerHelper 的信号，且 MultiTaskView 的生命周期通常短于 DWindowManagerHelper，这里需要确保 MultiTaskView 析构时断开连接，或者使用 QPointer 保护。
  现状：Qt 的 QObject::connect 在发送者 (DWindowManagerHelper) 销毁时会自动断开，但如果接收者 (this) 先销毁，Qt 5 的机制也能处理，但最好显式处理或在析构时确认无悬空引用风险。目前的写法在 Qt 中通常是安全的。

总结建议

1. 移除冗余信号发射：在 multitaskview.cpp 中，移除 setSupported 调用前的 Q_EMIT visibleChanged()，避免信号重复发射。
2. 优化初始化逻辑：在 MultiTaskView::init 中，尽量避免显式调用 setVisible，除非确定这是为了覆盖配置文件的默认值。建议只设置 setSupported，让属性系统根据 m_visible (来自配置) 和 m_isSupported (来自环境) 自动决定最终状态。
3. 增加空指针保护：在遍历 dockApplets 时，建议增加对 dockApplet 是否为空的检查，提高代码健壮性。